[WC-3435]: Image Cropper Design and feats#2280
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
60f8a9b to
2741159
Compare
… fix crop alignment Replace CSS-transform rotation with a pixel re-bake so the crop selection maps to the visible image, and add a blob-URL live preview so the rotation is visible before the deferred Mendix commit (Save). Reset now restores the true first-load original via an internal-change gate in useOriginalImage.
…used rotation state
…esign mode Simplify structure mode to a title row plus a single status/config row ([No attribute selected] vs config summary), dropping the icon and large image render. Rewrite design mode into a three-state preview driven by the bound image: static images render the real CropArea (non-interactive) with a config caption, while dynamic/unbound images show a placeholder glyph with [No image selected yet]. Extract shared describeConfig/aspectLabel into utils, add the cropper placeholder asset, declare the png module for TypeScript, and cover both editor surfaces with new specs.
…t fixes Terminology: - rename user-facing "Rotate" to "Flip" (toolbar, tooltips, enableFlip property) - rename "Black and white"/"B&W" toggle to "Grayscale" Toolbar: - single-row layout: flip-left, flip-right, grayscale, zoom slider, reset - flip buttons use SVG icons; square, icon-only styling - always-on native tooltips on every toolbar button Layout: - size widget to its image and render block-level so sibling widgets stack below - editor design-mode preview fills available width - remove redundant "Image cropper" label above the design-mode preview Behavior: - keep a color working image on flip so grayscale stays reversible; bake grayscale only into the committed file when the toggle is on - gate crop auto-commit on a genuine user drag so editing one cropper does not commit sibling instances in a list Tests: rename specs for flip/grayscale; add grayscale-reversibility and multi-instance commit regression specs.
2741159 to
1472687
Compare
This comment has been minimized.
This comment has been minimized.
Remove keepSelection and add MIN_CROP_PX min width/height so dragging on empty canvas starts a fresh selection instead of nudging the old one. A bare click (mousedown+mouseup, no drag) yields a ~0-size crop from react-image-crop that minWidth/minHeight does not catch, which would wipe the box. Guard it with isStrayCrop: crops below MIN_DRAW_FRACTION of the displayed image are ignored so the existing selection survives.
Reset previously cleared the selection, leaving no box. Restore the original bytes and re-seed the 80%-centered default via the extracted buildInitialCrop (now in utils/initialCrop). Two paths: when restoring changed the uri, CropArea's onLoad re-seeds against the correct dimensions; when nothing was edited the uri is unchanged and onLoad won't refire, so the container re-seeds directly from imageRef. Also mirror handleFlip and drive the live preview with the original bytes, so a stale rotated/flipped blob stops rendering after Reset.
Zoom now anchors on a fixed point (the crop-box center, captured when the zoom value changes and frozen while the box moves/draws), so scaling keeps that point on screen instead of drifting. The container owns the anchor and passes it to both CropArea's transformOrigin and the export math. Centralize the source-rect mapping into computeSourceRect, shared by cropImage and PreviewPane, so exported pixels match the on-screen framing. This also fixes a pre-existing off-center bug: the old pixelCrop.x / z math assumed a top-left origin while the CSS used center, giving the wrong region for any off-center crop. The read window is clamped into the image to guard zoom-out (z < 1) from reading off-canvas.
This comment has been minimized.
This comment has been minimized.
The buttons rotate the image by ±90° (via rotateImage), so "flip" was the wrong term. Rename it everywhere: the enableFlip XML property key becomes enableRotation, the rotate-left/rotate-right icon assets, CropToolbar props and aria-labels/titles, the container's handleRotate handler, and all affected tests, comments, and generated typings. Also polish the toolbar: render the "Zoom" label at regular weight (Atlas styles labels bold), and rename the Reset button caption to "Enable reset". The showResetButton property key is unchanged to avoid breaking existing app bindings.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
37dfe9b to
050b169
Compare
This comment has been minimized.
This comment has been minimized.
050b169 to
f4ecce1
Compare
This comment has been minimized.
This comment has been minimized.
Remove the showPreview / previewWidth / previewHeight properties and the PreviewPane component that rendered a live crop thumbnail next to the canvas. Drops the XML property group, editor hide-logic, SCSS rule, spec requirement, and the related test mock props. The usePreviewSrc hook (rotate/reset live feedback) and the Studio Pro editor preview machinery are unrelated and left intact.
…ar labels Add an Accessibility property group with 8 translatable textTemplate props (3 captions, 5 aria labels) covering the rotate, grayscale, reset, and zoom toolbar controls. Labels ship en_US/nl_NL defaults and are resolved to plain strings with English fallbacks before reaching the presentational components.
Replace the plain red 'No image' text with an informational placeholder: an inline info-circle icon plus 'No uploaded image to crop', laid out on a single row and colored with Atlas's semantic --brand-info token instead of the error red. The icon inherits the text color via currentColor.
| if (prevUri.current !== committedUri) { | ||
| prevUri.current = committedUri; | ||
| if (blobRef.current) { | ||
| revoke(); | ||
| setPreviewSrc(undefined); | ||
| } | ||
| } |
There was a problem hiding this comment.
URL.revokeObjectURL) called during render
revoke() triggers URL.revokeObjectURL, which is a side effect with an observable external result. React allows calling setState during render as a derived-state bailout, but external side effects in render violate render-purity. In React 18 concurrent mode an interrupted render can leave the blobRef mutation in place without completing the paired setPreviewSrc(undefined) call on the abandoned pass.
A useLayoutEffect keyed on committedUri avoids the issue — it fires synchronously after paint, like a synchronous effect, but keeps render pure:
| if (prevUri.current !== committedUri) { | |
| prevUri.current = committedUri; | |
| if (blobRef.current) { | |
| revoke(); | |
| setPreviewSrc(undefined); | |
| } | |
| } | |
| useLayoutEffect(() => { | |
| if (prevUri.current !== committedUri) { | |
| prevUri.current = committedUri; | |
| if (blobRef.current) { | |
| revoke(); | |
| setPreviewSrc(undefined); | |
| } | |
| } | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [committedUri]); // revoke is stable — intentionally omitted |
| case "portrait3x4": | ||
| return "3:4"; | ||
| case "custom": | ||
| return `${values.customAspectWidth}:${values.customAspectHeight}`; |
There was a problem hiding this comment.
"null:null" when custom dimensions are not yet set
ImageCropperPreviewProps.customAspectWidth / customAspectHeight are typed number | null. Before the user fills in values in Studio Pro the interpolation produces the string "null:null". The null case is not covered by any spec.
| return `${values.customAspectWidth}:${values.customAspectHeight}`; | |
| return `${values.customAspectWidth ?? 1}:${values.customAspectHeight ?? 1}`; |
Pull request type
New feature (non-breaking change which adds functionality)
Description
Initial development of the Image Cropper widget. This is a pre-release widget; the items below describe its current capabilities, not changes to a shipped version.
Cropping
Toolbar
Editor experience (Studio Pro)
Layout
Behavior
safeImageUriallowlistTesting
What should be covered while testing?